fix(postgrest): throw on multi-row result from maybeSingle when throw-on-error is enabled - #1180
fix(postgrest): throw on multi-row result from maybeSingle when throw-on-error is enabled#1180grdsdev wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Sequence Diagram(s)sequenceDiagram
participant PostgrestBuilder
participant PostgREST
participant PostgrestError
PostgrestBuilder->>PostgREST: Execute maybeSingle request
PostgREST-->>PostgrestBuilder: Return data or PGRST116 error
PostgrestBuilder->>PostgrestError: Parse details
PostgrestError-->>PostgrestBuilder: Identify zero-row or multiple-row result
PostgrestBuilder-->>PostgrestBuilder: Return nil or rethrow error
Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cac1794 to
f894afd
Compare
Coverage Report for CI Build 31187909468Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.3%) to 84.231%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
The following capabilities are marked
These may have been renamed, removed, or never registered. Please update the capability matrix. |
…-on-error is enabled
maybeSingle() swallowed PGRST116 unconditionally, so a query that matched
more than one row silently returned nil instead of surfacing the error --
hiding a real bug where the query should have been scoped to match at most
one row. Zero rows still returns nil; more than one row now throws, same as
single() already does.
PostgREST reports both cases with the same error code, distinguishable only
via the details message ("Results contain N rows, ..."), so maybeSingle now
inspects that to decide whether to swallow or rethrow.
Fixes #1170
maybeSingle()'s zero-vs-multiple-row detection only matched the "Results contain N rows" wording, but the real PostgREST server (used in the Linux integration CI job) instead sends "The result contains N rows", causing maybeSingle() to rethrow on zero rows and fail the maybeSingleReturnsNilOnZeroRows integration test. Extract the row count by scanning for the number preceding a "row"/"rows" token instead of matching a fixed prefix, so both wordings work.
cc7dfd3 to
2f307f5
Compare
What
maybeSingle()unconditionally swallowedPGRST116("no/too many rows"), so a query that unexpectedly matched more than one row silently returnednilinstead of surfacing an error — hiding a real bug where the query should have been scoped to match at most one row.nil(unchanged)PostgrestError, matching the behavior ofsingle()PostgREST reports both cases with the same
PGRST116code; the row count is only distinguishable via thedetailsmessage ("Results contain N rows, application/vnd.pgrst.object+json requires 1 row"), somaybeSingle()now inspects that to decide whether to swallow or rethrow.Fixes #1170
Depends on #1159
This fix relies on
PostgrestError.detailsactually decoding from the server response — which it doesn't today onmain(see #1159: the wire key is"details", but the property/synthesized decoding key wasdetail, so it silently dropped).The first commit on this branch (
fix(helpers): decode PostgREST error details field) mirrors #1159's change so this PR is self-contained and testable independently. Once #1159 merges intomain, rebasing this branch will produce a trivial no-op conflict on that one commit (identical change) — drop it and keep the second commit.Verification
swift test --filter "PostgRESTTests|HelpersTests"— 229 tests passswift package diagnose-api-breaking-changes origin/main— no breaking changes in any module./scripts/format.sh— no-opAcceptance criteria
maybeSingle()behavior